Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compiler.js): updated Testcafe dependency to 1.20 and fixed sourc… #98

Merged
merged 4 commits into from
Aug 8, 2022

Conversation

eroetman
Copy link
Contributor

…e to work with that version

#97

Copy link
Owner

@Arthy000 Arthy000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you revert the version change?

package.json Outdated Show resolved Hide resolved
src/compiler.js Outdated Show resolved Hide resolved
@Arthy000
Copy link
Owner

Arthy000 commented Aug 5, 2022

Hello @eroetman

Sorry for the delays, it's been a crazy week.

First of all, thanks for taking the time to fix this issue.
I've finally taken the time to further investigate the issue, and it looks like the optional compilerOptions parameter became mandatory, while the enum that references the names of the compiler still only contains "typescript".

This means that we can't provide

this.externalCompilers = [
  new TestcafeESNextCompiler(compilerOptions[CustomizableCompilers.esnext]),
  new TestcafeTypescriptCompiler(compilerOptions[CustomizableCompilers.typescript]),
];

because CustomizableCompilers.esnext doesn't exist.

However, providing CustomizableCompilers.typescript instead makes very little sense, because we are not working with typescript here.

We can't decide to not provide anything (as the parameter is now expected to be defined, which is what causes the issue in the first place)

For minimal impact on the code, I'd rather give the function an empty obect:

this.externalCompilers = [
  new TestcafeESNextCompiler({}),
  new TestcafeTypescriptCompiler(compilerOptions[CustomizableCompilers.typescript]),
];

This means that we keep using whatever TestCafe considers the default values of any option that could be passed down. Basically, nothing changed compared to the original code, which has been running long enough that I'm confident it works.

I'll add remarks on other parts of the code, but nothing major, just me nitpicking :)

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated
"@cucumber/cucumber-expressions": "^15.0.0",
"testcafe": "~1.18.0"
"@cucumber/cucumber": "^8.5.1",
"@cucumber/cucumber-expressions": "^16.0.0",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would have been necessary if we want to force the users to switch to version 16, but at the moment I'd rather keep it to version 15 so you can revert back to ^15.0.0

src/compiler.js Outdated Show resolved Hide resolved
@eroetman
Copy link
Contributor Author

eroetman commented Aug 7, 2022

My first adventure on GitHub.... It took some time because my main computer won't work as well with github as with my company source control system, will try to solve that coming week.
Thank you for taking the time to dive into this issue. I look forward to using gherkin with Testcafe 1.20 because then we can start using testcafé's API-testing possibility and maybe replace Supertest with this.

@Arthy000 Arthy000 merged commit 33dd883 into Arthy000:develop Aug 8, 2022
@eroetman eroetman deleted the feature/update-testcafe-1.20 branch August 8, 2022 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants